Skip to content

Conversation

@jeantil
Copy link
Contributor

@jeantil jeantil commented Aug 17, 2025

ScalaCompileMojo#getClasspathElements and ScalaTestCompileMojo#getClasspathElements compute the compilation and test compilation classpath from MavenProject#getCompileClasspathElements which is a List<String>.

Transforming from the ordered List<String> into an unordered Hashset<File> can change the order of classpath elements.

In my specific case, it broke the develocity cache key computation I'm using to improve Apache James build times.
In other cases it could lead to incorrect compilation or compilation errors ..

@slandelle
Copy link
Collaborator

Hey buddy !
Glad to see you here!
I’m on vacation, I’ll try to get a look next week but it might have to wait until the week after.

@jeantil
Copy link
Contributor Author

jeantil commented Aug 17, 2025

hello stephane ! I'm on vacation too !

There is no hurry I will have to wait for a release before I put this in james build anyway :D

enjoy your rest :D

Copy link
Collaborator

@slandelle slandelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a TreeSet feels very weird to me. TreeSet sorts by default with natural order, which I’m not even sure what it means for Files. It doesn’t conserve first insertion order.
What you want is a LinkedHashSet.
Lack of unit tests.

@jeantil
Copy link
Contributor Author

jeantil commented Aug 18, 2025

Sorry about that you are perfectly right. I changed the implementation to use a LinkedHashSet not much available on jdk8. I added a test which is red with both toSet and TreeSet

`getClasspathElements` computes the compilation and test compilation classpath from `MavenProject#getCompileClasspathElements` which is a `List<String>`.

Transforming from the ordered `List<String>` into an unordered `Hashset<File>` can change the order of classpath elements.

While this didn't cause a compilation issue in my case it could.
It did break the [develocity cache key computation](https://develocity.apache.org/c/unuozmm6ecqpc/nusnauq3l6a6u/goal-inputs?expanded=WyJuemRibm96bGFrZ2FnLWNsYXNzcGF0aGVsZW1lbnRzIiwibnpkYm5vemxha2dhZy1jbGFzc3BhdGhFbGVtZW50cy0wLWZpbGUtb3JkZXIiXQ) I'm using to improve Apache James build times.
@slandelle slandelle merged commit baebbdc into davidB:master Aug 19, 2025
1 check passed
@slandelle
Copy link
Collaborator

Hey @jeantil

4.9.6 is out with your fix. Sorry for the delay.

@jeantil
Copy link
Contributor Author

jeantil commented Sep 13, 2025

Thank you for letting me know ! the delay is perfectly normal :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants